fix: Nft chainId and current global chainId arent always the same #30517

gambinish · 2025-02-22T02:48:33Z

Description

Fixes a bug, where the network badge on the top of the nft in the nft details page did not correspond to the chain it belonged to. This resulted in navigating to send screen on a network that the NFT did not belong to.

This PR introduces two things:

Show correct NFT chain badge on NFT details screen, regardless of which network user is on
When navigating to the send flow for an NFT, where the network the NFT belongs to is not the globally selected chainId, change the network on behalf of the user so that the globally selected chainId is the same as the NFT being sent.

Related issues

Fixes:

Manual testing steps

Go to NFT Grid
Navigate to an NFT details screen on Ethereum Mainnet
Change global network to Linea
Should not see the network badge change on the NFT
Click send
See that network is changed on behalf of user from Linea back to Ethereum, where the NFT belongs

Screenshots/Recordings

Before

Screen.Recording.2025-02-21.at.7.19.21.PM.mov

After

Screen.Recording.2025-02-21.at.6.47.06.PM.mov

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

github-actions · 2025-02-22T02:48:47Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

metamaskbot · 2025-02-22T03:22:17Z

Builds ready [c2c44ea]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12
- content-script: 0
- offscreen: 0

Page Load Metrics (1605 ± 94 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	554	2104	1525	271	130
		domContentLoaded	1366	2124	1574	196	94
		load	1382	2140	1605	195	94
		domInteractive	23	183	66	43	21
		backgroundConnect	12	74	37	21	10
		firstReactRender	13	75	30	23	11
		getState	4	67	18	20	10
		initialActions	0	1	0	0	0
		loadScripts	952	1727	1173	172	83
		setupStore	7	74	16	19	9
		uiStartup	1556	2366	1817	192	92

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 577 Bytes (0.01%)
common: 0 Bytes (0.00%)

metamaskbot · 2025-02-24T19:39:23Z

Builds ready [03b877d]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12
- content-script: 0
- offscreen: 0

Page Load Metrics (1632 ± 50 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	408	1811	1509	369	177
		domContentLoaded	1390	1747	1593	104	50
		load	1441	1812	1632	104	50
		domInteractive	21	123	54	31	15
		backgroundConnect	12	85	41	25	12
		firstReactRender	13	76	31	23	11
		getState	5	54	14	13	6
		initialActions	0	1	0	0	0
		loadScripts	1006	1341	1175	96	46
		setupStore	7	59	13	13	6
		uiStartup	1593	2049	1845	117	56

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 577 Bytes (0.01%)
common: 0 Bytes (0.00%)

gambinish · 2025-02-24T20:28:49Z

ui/components/app/assets/nfts/nft-details/nft-details.tsx

+function NftDetails({ nft }: { nft: Nft }) {
+  const { chainId } = useParams();
+
+  return <NftDetailsComponent nft={nft} nftChainId={chainId ?? ''} />;
+}


Had to make this additional wrapper to satisfy the storybook ci test. It didn't like me using useParams in storybook and only wanted props (rather than an internal hook), so this wrapper allows me to bypass that issue while maintaining functionality.

metamaskbot · 2025-02-24T20:50:35Z

Builds ready [ec16cfd]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12
- content-script: 0
- offscreen: 0

Page Load Metrics (1625 ± 52 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	455	1827	1555	274	131
		domContentLoaded	1391	1809	1600	111	53
		load	1409	1830	1625	109	52
		domInteractive	24	128	49	30	14
		backgroundConnect	6	51	27	13	6
		firstReactRender	14	81	36	26	13
		getState	4	58	15	18	9
		initialActions	0	1	0	0	0
		loadScripts	985	1370	1177	97	46
		setupStore	7	70	18	20	10
		uiStartup	1600	2054	1853	112	54

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 708 Bytes (0.01%)
common: 0 Bytes (0.00%)

darkwing

This is great!

metamaskbot · 2025-02-25T00:27:13Z

Builds ready [76097d3]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12
- content-script: 0
- offscreen: 0

Page Load Metrics (1760 ± 69 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	358	2058	1631	422	203
		domContentLoaded	1458	2012	1730	141	68
		load	1471	2058	1760	145	69
		domInteractive	27	79	41	15	7
		backgroundConnect	9	79	31	21	10
		firstReactRender	15	76	31	23	11
		getState	4	70	17	19	9
		initialActions	0	1	0	0	0
		loadScripts	1035	1534	1296	120	58
		setupStore	7	71	26	24	11
		uiStartup	1660	2354	2002	163	78

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 708 Bytes (0.01%)
common: 0 Bytes (0.00%)

fix: Nft chainId and current global chainId arent always the same

c2c44ea

gambinish requested a review from darkwing February 22, 2025 02:48

gambinish requested a review from sahar-fehri February 22, 2025 02:48

metamaskbot added the team-assets label Feb 22, 2025

fix: Update nft-details unit tests with useParam mock

03b877d

gambinish added 3 commits February 24, 2025 11:52

fix: Storybook MemoryRouter for test

34830db

fix: modify chainId in react router mock

dd356da

refactor: Refactor component structure to satisfy storybook props

ec16cfd

gambinish commented Feb 24, 2025

View reviewed changes

gambinish marked this pull request as ready for review February 24, 2025 20:50

darkwing approved these changes Feb 24, 2025

View reviewed changes

Merge branch 'main' into fix/bug-nft-multichain-logo

76097d3

salimtb approved these changes Feb 25, 2025

View reviewed changes

gambinish added this pull request to the merge queue Feb 25, 2025

Merged via the queue into main with commit 78c5d2c Feb 25, 2025
79 checks passed

gambinish deleted the fix/bug-nft-multichain-logo branch February 25, 2025 01:18

github-actions bot locked and limited conversation to collaborators Feb 25, 2025

metamaskbot added the release-12.14.0 Issue or pull request that will be included in release 12.14.0 label Feb 25, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Nft chainId and current global chainId arent always the same #30517

fix: Nft chainId and current global chainId arent always the same #30517

gambinish commented Feb 22, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Feb 22, 2025

metamaskbot commented Feb 22, 2025

metamaskbot commented Feb 24, 2025

gambinish Feb 24, 2025 •

edited

Loading

metamaskbot commented Feb 24, 2025

darkwing left a comment

metamaskbot commented Feb 25, 2025

fix: Nft chainId and current global chainId arent always the same #30517

fix: Nft chainId and current global chainId arent always the same #30517

Conversation

gambinish commented Feb 22, 2025 • edited by github-actions bot Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Feb 22, 2025

metamaskbot commented Feb 22, 2025

metamaskbot commented Feb 24, 2025

gambinish Feb 24, 2025 • edited Loading

Choose a reason for hiding this comment

metamaskbot commented Feb 24, 2025

darkwing left a comment

Choose a reason for hiding this comment

metamaskbot commented Feb 25, 2025

gambinish commented Feb 22, 2025 •

edited by github-actions bot

Loading

gambinish Feb 24, 2025 •

edited

Loading